-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[common/buf] Add bytespools ownership for buf.Buffer #2336
[common/buf] Add bytespools ownership for buf.Buffer #2336
Conversation
Codecov ReportBase: 39.21% // Head: 39.19% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2336 +/- ##
==========================================
- Coverage 39.21% 39.19% -0.03%
==========================================
Files 617 617
Lines 36859 36871 +12
==========================================
- Hits 14454 14451 -3
- Misses 20816 20829 +13
- Partials 1589 1591 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@AkinoKaede For your comment in #1919 :
We can construct a scenario that a UDP packet exceeding 2048 bytes would be problematic:
V2Ray's UDP worker only accepts UDP payload with a 2048 bytes buffer, so for Socks5 and Dokodemo UDP inbound it would not be an issue. However for Trojan inbound this is not the case. A trojan inbound is a TCP inbound, which can receive more than 2048 bytes of a single UDP payload through TCP stream from another trojan client (not necessarily v2ray trojan client). In turn, v2ray's trojan inbound will write this large payload through dispatched link to the outbound. Let's see how current trojan inbound deals with it:
So how to solve this problem? A good solution would be to respect the semantic that a UDP packet is represented as a This functionality is exactly what this PR provides. By using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is appropriate. It is ready to be meregd.
ddb8530
to
44be94a
Compare
V2Ray has a powerful tool that's seldom used (only 2 references in whole project):
bytespool
.bytespool
has built in 4 pools of size2048
,8192
,32768
and131072
. The most frequently usedbuf.Buffer
only use the first 2048 byte pool. The other three pools are actually very suitable for payload of variable length that may exceed default 2048 bytes.bytespool
usesAlloc()
andFree()
pair to manage the bytes buffers. This PR extendsbuf.Buffer
to:bytespools
ownership, to make use of this two functions to provide a Buffer that can hold more size than 2048NewWithSize(size int32)
, to return a buffer with capacity having at least the given size (Under the hood it will choose one of the four pools to provide a bytes buffer).